Send full returnToUrl to oidc-auth-manager #648
Conversation
| </div> | ||
| <div class="container"> | ||
| <form method="post" action="/api/auth/select-provider"> | ||
| <form method="post"> |
There was a problem hiding this comment.
Always POST to self, so we don't lose query string.
| <label for="webid">WebID or server URL:</label> | ||
| <input type="text" class="form-control" name="webid" id="webid" | ||
| value="{{serverUri}}" /> | ||
| <input type="hidden" name="returnToUrl" id="returnToUrl" value="" /> |
There was a problem hiding this comment.
This one was always empty, so rather pointless. Instead, the query string should be used.
| '/api/auth/select-provider?returnToUrl=' + currentUrl | ||
| const locals = req.app.locals | ||
| const currentUrl = util.fullUrlForReq(req) | ||
| const loginUrl = `${locals.host.serverUri}/api/auth/select-provider?returnToUrl=${encodeURIComponent(currentUrl)}` |
There was a problem hiding this comment.
Escaping was definitely important here (for query strings and such).
| <meta charset="UTF-8"> | ||
| <script> | ||
| window.location.href = "${url}" + window.location.hash | ||
| window.location.href = "${url}" + encodeURIComponent(window.location.hash) |
There was a problem hiding this comment.
So it is the client who appends the hash (and only the client can do that).
| res.header('Content-Type', 'text/html') | ||
|
|
||
| let currentUrl = util.fullUrlForReq(req) | ||
| req.session.returnToUrl = currentUrl |
There was a problem hiding this comment.
We can't rely on session, because the server cannot see the hash.
There was a problem hiding this comment.
This fixed mechanism (not relying on session) will still work on direct requests by the browser (like, requesting an image or a pdf file or whatever), not mediated by a client?
There was a problem hiding this comment.
If we end up in this code, we are in the process of rendering an HTML error page. So if that's the behavior we exhibit in response to a browser directly requesting an image (I hope not), it's broken for another reason already 😄
There was a problem hiding this comment.
Ahh, I see. Makes sense.
Addresses the node-solid-server part of #571.
RubenVerborgh
force-pushed
the
fix/redirect-hash
branch
from
8e3c6d0 to
299f5d6
Compare
RubenVerborgh
force-pushed
the
fix/redirect-hash
branch
from
b349038 to
e7be5dd
Compare
RubenVerborgh
force-pushed
the
fix/redirect-hash
branch
from
e7be5dd to
3da620e
Compare
3 participants
Addresses the node-solid-server part of #571.
oidc-auth-manager still loses the URL we send, so fixes on that side are needed to fully solve #571. Tracking that in nodeSolidServer/oidc-auth-manager#17.